-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename JsonRpcProxyFactory and related types #12588
Conversation
57f9b9a
to
440a59d
Compare
@tortmayr would you mind resolving the conflict on this one? |
440a59d
to
5e7343f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two wordings, otherwise blissfully boring.
CHANGELOG.md
Outdated
@@ -11,7 +11,10 @@ | |||
- [repo] with the upgrade to Inversify 6.0, a few initialization methods were adjusted. See also [this migration guide entry](https://github.com/eclipse-theia/theia/blob/master/doc/Migration.md#inversify-60). Additionally, other changes include: [#12425](https://github.com/eclipse-theia/theia/pull/12425) | |||
- The type expected by the `PreferenceProxySchema` symbol has been changed from `PromiseLike<PreferenceSchema>` to `() => PromiseLike<PreferenceSchema>` | |||
- The symbol `OnigasmPromise` has been changed to `OnigasmProvider` and injects a function of type `() => Promise<IOnigLib>` | |||
- The symbol `PreferenceTransactionPrelude` has been changed to `PreferenceTransactionPreludeProvider ` and injects a function of type `() => Promise<unknown>` | |||
- The symbol `PreferenceTransactionPrelude` has been changed to `PreferenceTransactionPreludeProvider` and injects a function of type `() => Promise<unknown>` | |||
- [rpc] Replaced name suffixes classes and types that were still referencing the old rpc protocol. From `JsonRpc*` to `Rpc*`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...name suffixes classes and types...
CHANGELOG.md
Outdated
- The symbol `PreferenceTransactionPrelude` has been changed to `PreferenceTransactionPreludeProvider ` and injects a function of type `() => Promise<unknown>` | ||
- The symbol `PreferenceTransactionPrelude` has been changed to `PreferenceTransactionPreludeProvider` and injects a function of type `() => Promise<unknown>` | ||
- [rpc] Replaced name suffixes classes and types that were still referencing the old rpc protocol. From `JsonRpc*` to `Rpc*`. | ||
- Old classes and types are still available but haven been deprecated and might get removed in future releases [#12588](https://github.com/eclipse-theia/theia/pull/12588) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and will be removed...
- Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely a good idea to give adopters a grace period. Complementary website PR: eclipse-theia/theia-website#422 Fixes #12581 Contributed on behalf of STMicroelectronics
5e7343f
to
da390fc
Compare
Thanks for the review. I have rebased and adapted the changelog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
What it does
JsonRpcProxyFactory
and related types toRpcProxyFactory
(Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol.By simply using the
Rpc
suffix the class names are less misleading and protocol agnostic.JsonRpc*
namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks.Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely
a good idea to give adopters a grace period.
Complementary website PR: eclipse-theia/theia-website#422
Fixes #12581
Contributed on behalf of STMicroelectronics
How to test
Nothing particular to test. Everything should work as expected
Review checklist
Reminder for reviewers